components/core/CMakeLists.txt (1)

Line range hint 102-117: Consider using CMake variables for library versions.

To improve maintainability, consider defining the versions at the top of the CMakeLists.txt file.

Here's a suggested implementation:

+ # Library versions
+ set(FMT_VERSION "8.0.1")
+ set(SPDLOG_VERSION "1.9.2")

- find_package(fmt 8.0.1 EXACT REQUIRED)
+ find_package(fmt ${FMT_VERSION} EXACT REQUIRED)

- find_package(spdlog 1.9.2 EXACT REQUIRED)
+ find_package(spdlog ${SPDLOG_VERSION} EXACT REQUIRED)

Configuration used: CodeRabbit UI
Review profile: CHILL

#!/bin/bash
# Description: Check spdlog's fmt version requirements in their release notes
gh api repos/gabime/spdlog/releases/tags/v1.9.2 --jq .body | grep -i "fmt"

#!/bin/bash
# Let's check spdlog's CMake files for fmt version requirements
rg -A 5 "find_package\(fmt" 

# Also check for any comments or documentation about fmt version requirements
rg -i "fmt.*(version|require)" 

# Look for fmt version mentions in CMake files
fd "CMakeLists.txt" --exec grep -i "fmt.*version"

#!/bin/bash
# Let's check spdlog's version requirement and any related comments
rg -A 5 "find_package\(spdlog" 

# Check for any installation scripts that might have version information
fd "spdlog" --exec grep -i "version"

# Look for any documentation about version compatibility
rg -l "spdlog.*fmt" README.md docs/ 2>/dev/null || true

components/core/CMakeLists.txt (1)

62-64: Consider enhancing the warning message for macOS users

The warning message could be more specific about why library version pinning isn't supported on macOS and what potential issues users might encounter.

-    message(AUTHOR_WARNING "Use the system provided library version for building,
-    as library version pinning is not supported on macOS.")
+    message(AUTHOR_WARNING "Use the system provided library version for building on macOS. "
+            "Library version pinning is not supported due to macOS system library management "
+            "and potential conflicts with system-wide dependencies.")

Configuration used: CodeRabbit UI
Review profile: CHILL

#!/bin/bash
# Description: Verify spdlog usage and version requirements
# Check for any explicit spdlog version requirements in other CMake files
fd -e cmake -e txt -x rg -A 3 "find_package\(spdlog" {}

# Check for any direct spdlog includes to ensure we're not bypassing version checks
rg -t cpp -t hpp "include.*spdlog" -A 2

#!/bin/bash
# Check for any explicit spdlog version requirements in other CMake files
fd -e cmake -x rg -A 3 "find_package\(spdlog" {}

# Check for any direct spdlog includes in source files
rg -t cpp "include.*spdlog" -A 2

# Check for any fmt package requirements as they should be compatible
fd -e cmake -x rg "find_package\(fmt" {}

# Look for spdlog configuration or version settings
rg "spdlog.*VERSION|VERSION.*spdlog" -t cmake

#!/bin/bash
# Description: Verify fmt version compatibility across the codebase
# Check for any explicit fmt version requirements in other CMake files
fd -e cmake -e txt -x rg -A 3 "find_package\(fmt" {}

#!/bin/bash
# Check for spdlog package requirements and version specifications
fd -e cmake -e txt -x rg -A 5 "find_package\(spdlog" {}

# Search for any fmt version references in the entire codebase
rg "fmt.*version|fmt.*[0-9]+\.[0-9]+\.[0-9]+" --type cmake

# Look for any dependency documentation
fd "README|DEPENDENCIES" -e md -e txt -x cat {}

#!/bin/bash
# Check for spdlog version specifications
rg "FIND_SPDLOG_PACKAGE_ARGS" --type cmake -A 3 -B 3

# Search for any spdlog version requirements or compatibility notes
rg "spdlog.*[0-9]+\.[0-9]+" --type cmake

components/core/CMakeLists.txt (2)

Line range hint 119-129: Enhance error message clarity for static library builds.

The error handling is good, but the error message for static builds could be more specific.

Consider this improvement:

-            `components/core/tools/scripts/lib_install/<dist>/install-packages-from-source.sh`")
+            `components/core/tools/scripts/lib_install/<dist>/install-packages-from-source.sh` where <dist> is your distribution name (e.g., ubuntu, centos)")

---

144-152: Consider adding version upper bounds for fmt.

While the minimum version requirements are set correctly, consider adding upper bounds to prevent potential future incompatibilities.

Consider modifying the find_package call:

-find_package(fmt ${FMT_MIN_VERSION} REQUIRED)
+find_package(fmt "${FMT_MIN_VERSION}...${FMT_MAX_VERSION}" REQUIRED)

Where FMT_MAX_VERSION would be set based on the spdlog version:

if(spdlog_VERSION VERSION_GREATER_EQUAL "1.15.0")
    set(FMT_MAX_VERSION "11.99.99")
# ... and so on

Configuration used: CodeRabbit UI
Review profile: CHILL

components/core/CMakeLists.txt (1)

144-152: Improve error message clarity.

The error message should align with the note that indicates only static libraries are attempted.

-    message(FATAL_ERROR "Could not find static libraries for fmt")
+    message(FATAL_ERROR "Could not find static library for fmt. This project requires static linking for fmt.")

Configuration used: CodeRabbit UI
Review profile: CHILL

components/core/CMakeLists.txt (2)

110-128: Add documentation for version selection rationale.

Consider adding a comment explaining why 1.9.0 was chosen as the minimum spdlog version. Also, enhance the error message for static libraries to guide users more specifically.

Apply this improvement:

 # Specify the minimum required spdlog version (can be overridden by the user)
+# Note: Version 1.9.0 is chosen as the minimum because it's the earliest version
+# that supports the required fmt library features and API compatibility.
 set (spdlog_MIN_VERSION "1.9.0" CACHE STRING "Minimum required version of spdlog")

---

144-168: Improve documentation and error message consistency.

The implementation is solid, but consider these improvements:

1. Make the static library requirement more explicit in the comment
2. Standardize error message format across all version checks

Apply these improvements:

 # Find fmt with the compatible version
-# NOTE:
-# - We only try to link to the static library
+# NOTE: We require static linking for fmt to ensure ABI compatibility
+# with the statically linked spdlog library
 find_package(fmt ${FMT_MIN_VERSION} REQUIRED)

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro